Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store original device layout was scraped with #1474

Merged
merged 26 commits into from
Oct 19, 2023

Conversation

rcj-siteimprove
Copy link
Contributor

@rcj-siteimprove rcj-siteimprove commented Oct 2, 2023

We should store the device used to scrape the layout to avoid using a layout that no longer applies if another device is afterwards used when auditing.

This PR includes:

  • Passing the device along when constructing any part of the document so that it can be stored on the elements
  • Weakly storing bounding box by device in a Cache on the element
  • Replacing direct box-accessing by function to get it by device
  • Enabling sending the device to tests through jsx
  • Updating the tests that depends on the box
  • Adding type parameter for serialization options to Serializer interface
  • Elements are serialized with serialization options containing the device for which the box should be serialized

@rcj-siteimprove
Copy link
Contributor Author

!pr extract

@rcj-siteimprove rcj-siteimprove marked this pull request as ready for review October 10, 2023 11:07
@rcj-siteimprove rcj-siteimprove requested review from a team and Jym77 October 10, 2023 11:07
@rcj-siteimprove rcj-siteimprove changed the title Store original device Store original device layout was scraped with Oct 10, 2023
@rcj-siteimprove
Copy link
Contributor Author

!pr debgraph

@rcj-siteimprove
Copy link
Contributor Author

!pr depgraph

@rcj-siteimprove
Copy link
Contributor Author

!pr extract

@rcj-siteimprove rcj-siteimprove self-assigned this Oct 12, 2023
@rcj-siteimprove rcj-siteimprove added the minor Backwards-compatible change that touches public API label Oct 12, 2023
Copy link
Contributor

@Jym77 Jym77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, some clean up and possible improvements.

.changeset/chilled-laws-hammer.md Outdated Show resolved Hide resolved
packages/alfa-dom/src/h.ts Outdated Show resolved Hide resolved
packages/alfa-dom/src/jsx.ts Show resolved Hide resolved
packages/alfa-dom/src/node.ts Outdated Show resolved Hide resolved
packages/alfa-dom/src/node.ts Outdated Show resolved Hide resolved
packages/alfa-dom/src/node/document.ts Outdated Show resolved Hide resolved
packages/alfa-dom/src/node/element.ts Outdated Show resolved Hide resolved
packages/alfa-dom/src/node/fragment.ts Fixed Show fixed Hide fixed
packages/alfa-web/src/page.ts Fixed Show fixed Hide fixed
@rcj-siteimprove
Copy link
Contributor Author

!pr extract

@rcj-siteimprove
Copy link
Contributor Author

!pr extract

@rcj-siteimprove rcj-siteimprove added major Backwards-incompatible change that touches public API and removed minor Backwards-compatible change that touches public API labels Oct 19, 2023
Copy link
Contributor

@Jym77 Jym77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

@rcj-siteimprove rcj-siteimprove added this pull request to the merge queue Oct 19, 2023
Merged via the queue into main with commit 6231b55 Oct 19, 2023
6 checks passed
@rcj-siteimprove rcj-siteimprove deleted the store-original-device branch October 19, 2023 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major Backwards-incompatible change that touches public API
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants